feat(agentex): DM unlinked Slack users a link to connect their account - #412

…ink, stop leaking ids (#414)

Four fixes to the identity-link work. Three were found by using it — the
config surface
was too large, the connect link was somewhere people don't look, the
confirmation page
printed internal ids — and the fourth is a P1 the first one introduced,
caught in review.

---

# 1. Config surface: 8 vars → 0

#409 → #412 accumulated **eight** environment variables. One was a
hazard, one a trap,
six were knobs nobody has ever turned.

```
IDENTITY_LINK_SESSION_COOKIE_NAME    ->  derived from the delegation allowlist
IDENTITY_LINK_REQUIRE_EMAIL_MATCH    ->  enables itself when the scope exists
IDENTITY_LINK_NONCE_TTL              ->  constant
IDENTITY_LINK_MAX_DMS                ->  constant
IDENTITY_LINK_CACHE_TTL              ->  constant
IDENTITY_LINK_NEGATIVE_CACHE_TTL     ->  constant
IDENTITY_LINK_FALLBACK_TTL_DAYS      ->  constant
SLACK_LINK_OFFER_COOLDOWN_S          ->  constant
```

**No behavior change at current settings** — every constant equals the
default it
replaced.

## The hazard: a cookie name with two sources of truth

```
identity_link_service:  IDENTITY_LINK_SESSION_COOKIE_NAME         default _identityJwt
delegation_headers:     AGENTEX_DELEGATION_SESSION_COOKIE_NAMES   default _identityJwt
```

`acting_headers()` emits a `Cookie` header that
`build_delegation_headers` filters down
to its allowlist. Had the two disagreed, the credential would be
**stripped in transit**
— every linked turn silently losing its acting identity, while the link
sat in the
database looking stored, valid and healthy.

Now derived, so divergence is unrepresentable rather than warned about
in a comment. An
empty allowlist returns `None` and `acting_headers()` refuses, since
emitting a
credential that will certainly be stripped is worse than admitting we
can't act.

## The trap: a flag that had to move in lockstep with a Slack scope

The email check needs `users:read.email`, which isn't granted. The flag
kept it off
until then — but flag and scope had to be flipped **together**: the flag
alone refused
*every* link, the scope alone protected nothing.

It now enables itself: enforces whenever Slack answers with an email,
stands down when
it won't. Granting the scope switches the protection on by itself.

⚠️ This inverts the unverifiable case from refuse to allow — weaker,
deliberately. With
no flag to distinguish "scope missing" from "Slack had a bad minute",
failing closed
would make linking fail at random. The gap isn't attacker-reachable:
nobody outside our
infrastructure influences whether *our* Slack lookup succeeds. Against
what's deployed
today (flag off, verifying nothing) it's strictly stronger.

## What remains

| Variable | Why |
|---|---|
| `AGENTEX_CREDENTIAL_ENCRYPTION_KEY` | Secret, no default possible |
| `SLACK_GATEWAY_PUBLIC_BASE_URL` | Deployment-specific; the feature's
on-switch |
| `SLACK_GATEWAY_REQUIRE_LINKED_USER` | Pre-existing, a real product
choice |

---

# 2. The connect link was invisible

The first real offer in production was **delivered correctly and
reported as never
received**. Both were true: `chat.postMessage` returned `ok` and
`conversations.history` confirmed the message in the DM channel — and
Slack files bot
conversations under **Apps**, not in the Direct messages list. "I've
DM'd you a link"
pointed at the one place it wasn't.

```
before                                  after
ephemeral: "I've DM'd you a link —      ephemeral: "<Connect your SGP account>
            check your DMs"                         …also sent to <our DM>"
clicks to connect: 2, if you find it    clicks to connect: 1
```

## Why this doesn't weaken anything

An ephemeral has **exactly the same audience as a DM**: rendered for one
user, absent
from channel history and search. The exposure argument that made this
DM-only never
applied to an ephemeral — so routing someone through a conversation they
can't find, to
click a link we could hand them directly, bought nothing.

The DM stays because ephemerals are **transient**: reload before
clicking and it's gone,
and the offer cooldown would then block a retry for an hour. So the
ephemeral carries
the link *plus* a deep link to the DM as the durable copy.

## The invariant, stated properly

> The nonce is a bearer token. It may go anywhere **exactly one person
can see it** (the
> user's DM, an ephemeral addressed to them) and **nowhere that lands in
channel
> history**.

The test that asserted *"never in a payload addressed to the origin
channel"* now
asserts *"never in a `chat.postMessage` outside the user's own DM"*.
That matters — the
old wording would have blocked this change while the actual risk was
never touched by
it. A test encoding the implementation rather than the property blocks
correct changes.

## Also

- `conversations.open` moved above the send-cap check (idempotent; both
branches need
  the channel id).
- **Past the DM cap the user still gets the live link** — the cap limits
DMs, not what
  we can show the person in front of us.
- `slack.com/app_redirect`, not `slack://`, which fails on the web
client.

---

# 3. The connect page printed internal ids

It fell back to raw identifiers when it couldn't name an identity:

```python
slack_who = link_request.display_name or link_request.external_user_id  # U0B01457V24
<dd>{email or sgp_user_id}</dd>                                         # 5da8f784-…
```

That leaks an internal id and buys nothing for it. The two identity rows
exist so the
person clicking can answer *"is this **my** Slack account?"* and stop if
it isn't — the
only defence against a link forwarded to them. Nobody recognises their
own Slack member
id or SGP uuid, so the fallback never made that question answerable. It
made an
unanswerable question **look** answered, which is worse than showing
nothing.

- Neither side ever falls back to an id; an unnameable identity renders
a placeholder.
- A missing Slack name is now **re-read live** — a transient Slack
failure when the
nonce was minted shouldn't permanently degrade the page, and
`users.info` needs only
  `users:read`, which is granted.
- When either side is unnamed, the caution changes from *"if either name
above isn't
you, don't continue"* to saying the match can't be confirmed here.
Claiming someone
  verified something they couldn't is the actual harm.
- The success page likewise stops printing the uuid when there's no
email.

---

# 4. A multi-name cookie allowlist got narrowed (P1, introduced by §1)

Deriving the cookie name fixed one hazard and introduced another.
`session_cookie_name()`
returned only the **first** entry of
`AGENTEX_DELEGATION_SESSION_COOKIE_NAMES`, and the
read path matched against that alone — so with a multi-name allowlist, a
session carried
by any later name was rejected. Linking failed with *"couldn't read your
session"* for a
cookie the delegation layer would have forwarded quite happily.

The allowlist is the set of names a deployment treats as valid sessions,
so one arriving
under the second entry is exactly as legitimate as the first.

Reading and writing genuinely differ, so they're now separate functions
rather than one
doing double duty — which is how the bug got in:

```
session_cookie_names()   every accepted name, in preference order   (READ)
session_cookie_name()    the single canonical name to emit under    (WRITE)
```

Allowlist order beats header order when several are present, so the
stored credential is
the canonical cookie whenever it's there — which is also the name we
emit under, keeping
the common case exact. Emitting a value that arrived under a later name
as the canonical
one is safe: the credential is a session JWT, validated downstream on
its contents, not
on its label. Flagged in the docstring — if a downstream ever became
name-sensitive this
would need the originating name stored alongside the credential, which
is a schema change
and not worth making speculatively.

---

## Testing

**26 new, 6 rewritten.** The rewrites are the interesting ones — they
invert assertions
that encoded superseded designs:

| Was | Now |
|---|---|
| nonce must **not** appear in the ephemeral | nonce **must**
(single-viewer) |
| nothing addressed to the origin channel carries it | nothing
**broadcast** carries it |
| email flag off → no Slack call | check always runs; scope decides |
| unreadable email fails closed | fails open, loudly logged |

New coverage: the cookie name follows the delegation allowlist and the
emitted cookie
survives the delegation filter under a non-default name;
`acting_headers` refuses when
delegation is off; the ephemeral carries the link and points at the
durable copy; an
unreachable DM offers nothing.

**708 unit tests pass.** The 14 Redis integration tests still pass
against a real Redis
after the TTL constants moved. `ruff` clean.

## Not fixed here

The nonce is still 10 minutes — sized assuming prompt discovery, which
this incident
showed was optimistic. With the link now inline the find-it delay
largely disappears,
so I'd rather see whether it's still a problem than change two things at
once.

🤖 Generated with [Claude Code](https://claude.com/claude-code)


<!-- greptile_comment -->

<h3>Greptile Summary</h3>

The PR simplifies identity-link configuration, improves Slack link
discoverability, removes raw identifiers from confirmation pages, and
fixes session extraction for multi-cookie delegation allowlists.

- Derives identity-link cookie handling from the delegation allowlist
and accepts sessions under any configured cookie name.
- Places the connection URL in the user-specific ephemeral response
while retaining the DM as a durable copy.
- Replaces internal identifier fallbacks with explicit unknown-identity
messaging and performs best-effort profile refreshes.
- Converts identity-link TTLs, limits, and cooldowns from environment
settings to fixed defaults.

<details><summary><h3>Confidence Score: 5/5</h3></summary>

The PR appears safe to merge because the previously reported
multi-cookie allowlist failure is fixed and no blocking eligible failure
remains.

No blocking failure remains.
</details>


<details><summary><h3>Important Files Changed</h3></summary>




| Filename | Overview |
|----------|----------|
| agentex/src/api/routes/integrations.py | Accepts every allowlisted
session-cookie name, adds best-effort identity naming and self-enabling
email comparison, and removes raw identifier fallbacks. |
| agentex/src/domain/services/identity_link_service.py | Derives
accepted and emitted cookie names from the delegation allowlist and
refuses acting headers when cookie delegation is disabled. |
| agentex/src/domain/services/link_nonce_service.py | Replaces nonce
lifetime and DM-send environment settings with constants matching their
former defaults. |
| agentex/src/domain/use_cases/slack_gateway_use_case.py | Surfaces the
bearer link in a user-specific ephemeral response, retains a durable DM
copy, and adds DM deep links. |
| agentex/tests/unit/api/test_integrations_routes.py | Adds regression
coverage for later allowlisted cookies, identity placeholders, live
Slack-name recovery, and email comparison behavior. |
| agentex/tests/unit/services/test_identity_link_service.py | Verifies
canonical cookie derivation, delegation forwarding, and
disabled-delegation behavior. |
| agentex/tests/unit/use_cases/test_slack_gateway_use_case.py | Verifies
that connection links remain single-viewer, discoverable, durable
through DM delivery, and available after the DM cap. |

</details>


<details><summary><h3>Flowchart</h3></summary>

```mermaid
%%{init: {'theme': 'neutral'}}%%
flowchart LR
    Request[Authenticated link confirmation] --> Parse[Parse inbound cookies]
    Allowlist[Delegation cookie allowlist] --> Parse
    Parse --> Select[Select first present allowlisted cookie]
    Select --> Store[Encrypt and store session JWT]
    Store --> Resolve[Resolve linked Slack identity]
    Allowlist --> Canonical[Choose canonical first cookie name]
    Resolve --> Emit[Emit JWT under canonical cookie name]
    Emit --> Delegate[Delegation filter forwards acting-user cookie]
```
</details>

<sub>Reviews (4): Last reviewed commit: ["fix(agentex): accept a session
under
any..."](bbceae9)
| [Re-trigger
Greptile](https://app.greptile.com/api/retrigger?id=58328790)</sub>

<!-- /greptile_comment -->

---------

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>